<html>
<head><meta charset="utf-8"><title>Limit self-args in generic derive code? · t-compiler · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/index.html">t-compiler</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html">Limit self-args in generic derive code?</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="223053995"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223053995" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Björn Steinbrink <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223053995">(Jan 17 2021 at 17:34)</a>:</h4>
<p>I noticed that the code generated by e.g. #[derive(PartialEq)] is not really good for things like Option&lt;char&gt;, i.e. an enum with two variants and a non-zero niche value. One reason for this is the duplicated discriminant generation (once for the equality check and once for the match) that LLVM can't always eliminate. Additionally, LLVM trips over the "if true &amp;&amp; ..." part, the extra BBs/conditional jumps somehow break some optimizations. I have a WIP fix for both (tests are still missing), but  I wonder whether we could simplify the code by limiting self args to two. Currently the code supports an arbitrary number of self args, which seems unnecessary. Is there anything that actually needs that or can I simplify the code there?</p>



<a name="223054017"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223054017" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Joshua Nelson <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223054017">(Jan 17 2021 at 17:35)</a>:</h4>
<p>what do you mean by 'an arbitrary number of self args'?</p>



<a name="223054323"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223054323" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Björn Steinbrink <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223054323">(Jan 17 2021 at 17:42)</a>:</h4>
<p>"self_args" is the name used in the code. These are the args that have type Self in the method definition. e.g. Hash has one self arg, PartialEq has two, etc. The generic deriving code generates a match to generate individual arms for each variant, iff all self args have the same variant, plus one for mismatching variants, is applicable</p>



<a name="223054402"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223054402" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Joshua Nelson <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223054402">(Jan 17 2021 at 17:44)</a>:</h4>
<p>this sounds like the sort of question to ask the reviewer of the code</p>



<a name="223054403"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223054403" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Joshua Nelson <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223054403">(Jan 17 2021 at 17:44)</a>:</h4>
<p>I don't think I can answer it without seeing the changes</p>



<a name="223073184"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223073184" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Björn Steinbrink <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223073184">(Jan 18 2021 at 01:13)</a>:</h4>
<p>Thanks, but in that case, I'd like to get feedback from someone who's familiar with that area of the compiler before I put in a significant amount of time to refactor it just to be told by a reviewer that the refactoring was pointless due to a use case that was not obvious to me</p>



<a name="223073879"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223073879" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223073879">(Jan 18 2021 at 01:32)</a>:</h4>
<p>I think if we observe benefits from special casing N self-args, we would likely land that PR - even if the generic code path is actually necessary.</p>



<a name="223111469"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223111469" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Björn Steinbrink <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223111469">(Jan 18 2021 at 12:04)</a>:</h4>
<p>I ran a build and tests with <code>assert!(self_args.len() &lt; 3)</code> which succeeded, so I guess now the better question is whether that code is exposed or reused somewhere outside the compiler, maybe any of the proc macro related crates?</p>



<a name="223121495"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223121495" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Joshua Nelson <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223121495">(Jan 18 2021 at 13:50)</a>:</h4>
<p>The compiler API is unstable, so if all the tests pass it's fine to make the change. Have you tried a derive with more than 3 variants to make sure it works?</p>



<a name="223131385"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Limit%20self-args%20in%20generic%20derive%20code%3F/near/223131385" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Björn Steinbrink <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Limit.20self-args.20in.20generic.20derive.20code.3F.html#223131385">(Jan 18 2021 at 15:08)</a>:</h4>
<p>Did you mean self args? Since variants don' factor into this AFAICT. Either way, there are no derives with more than 2 self args, but some have more args in total (args are split between Self type and others). Also my other changes need to take into account the number of fieldless variants in particular and differentiate between enums with varying numbers of variants, so yeah, I tested that with enums with up to 4 variants in different combinations.</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>